First task solution#2682
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there is a critical, blocking issue in your implementation that prevents the function from working as required. In src/formatDate.js, the call date.split(baseElement) uses baseElement without ever defining it (ESLint no-undef and a runtime ReferenceError), so the code cannot run. Per the task, the input separator must come from the fromFormat array, with its last element representing the separator; you should derive something like const inputSeparator = fromFormat[fromFormat.length - 1]; and then use date.split(inputSeparator) instead. Closely related, your current logic ignores this separator part of fromFormat and only uses fromFormat to find positions of DD, MM, and YY/ YYYY, which means the function does not fully honor the requirement that fromFormat describes both order and separator of the input date. Once you fix the undefined variable and properly use the fromFormat separator when splitting, your reordering and year conversion logic (including the YY ↔ YYYY rules) should align well with the specification. As a minor improvement, consider clearer names like inputSeparator and outputSeparator to make the intent of each step easier to follow. Your core idea is solid, so with these changes you’ll have a robust solution that matches the task requirements.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| function formatDate(date, fromFormat, toFormat) { | ||
| // write code here | ||
| const changeElement = toFormat[toFormat.length - 1]; | ||
| const parts = date.split(baseElement); |
There was a problem hiding this comment.
baseElement is used here but never defined anywhere in the function or file. You should derive the input separator from fromFormat (e.g., its last element) and use that variable in split to correctly parse the date and avoid a ReferenceError.
| */ | ||
| function formatDate(date, fromFormat, toFormat) { | ||
| // write code here | ||
| const changeElement = toFormat[toFormat.length - 1]; |
There was a problem hiding this comment.
Here you correctly read the output separator from toFormat, but you never similarly read the input separator from fromFormat. According to the task, fromFormat also includes the separator, so you should use its last element to determine how to split the date string.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there is a critical bug in how you derive the input separator from fromFormat, and this prevents the function from correctly parsing the date string for all specified examples. Right now, baseElement is taken as fromFormat[toFormat.length - 1], which can be a format token like 'DD' instead of the actual separator, so date.split(baseElement) will not work as required for valid inputs such as ['YYYY','MM','DD','-']. You need to change the separator derivation so it always uses the last element of fromFormat, for example const baseElement = fromFormat[fromFormat.length - 1];, and then keep using that to split the incoming date string [CHECKLIST ITEM #4, CHECKLIST ITEM #5]. Once that is fixed, your logic for reordering parts and converting between YY and YYYY appears to meet the task requirements. As a minor improvement, consider renaming baseElement and changeElement to something like inputSeparator and outputSeparator to make the intent clearer, though this is not blocking. Overall, you are very close; fixing the separator handling so it matches the fromFormat specification should make your solution correct and complete.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| */ | ||
| function formatDate(date, fromFormat, toFormat) { | ||
| // write code here | ||
| const baseElement = fromFormat[toFormat.length - 1]; |
There was a problem hiding this comment.
baseElement should be derived from the separator in fromFormat, which is always its last element. Using fromFormat[toFormat.length - 1] gives the wrong index whenever fromFormat and toFormat differ (e.g. ['YYYY','MM','DD','-'] and ['DD','MM','YYYY','.'] results in 'DD' instead of '-'). Change this to something like const baseElement = fromFormat[fromFormat.length - 1]; so the input date is split by the correct separator as required.
| // write code here | ||
| const baseElement = fromFormat[toFormat.length - 1]; | ||
| const changeElement = toFormat[toFormat.length - 1]; | ||
| const parts = date.split(baseElement); |
There was a problem hiding this comment.
Once you fix the separator derivation above, this split will work correctly with the formats described in the task (the date string must be split using the separator defined in fromFormat). Right now, because baseElement can be a format token like 'DD' instead of the actual separator, parts will be incorrect and the function will fail to meet the specification.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution because your formatDate implementation correctly handles all required behaviors: it derives the input separator from the last element of fromFormat, splits the input date with it, and uses the last element of toFormat to join the output string, matching the separator rules [CHECKLIST ITEM #4 and #5]. You accurately locate the day, month, and year positions based on fromFormat, reorder them for toFormat, and implement the YYYY ↔ YY conversions with the correct 20YY/19YY rule, which aligns with all specified examples. Your code focuses on the core logic without adding unnecessary validation, which is appropriate given the assumption that inputs are always well-formed.
One minor improvement opportunity is naming clarity—for example, a name like yearIndex instead of fromYearIndex could make the role of the variable more immediately obvious, but this is stylistic and your current naming is still understandable. Overall, this is solid, clean work that shows you carefully followed the requirements and previous feedback. Nice job implementing the rules precisely and keeping the solution straightforward and readable.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
No description provided.